feat: Propagate W3C + Sentry headers in Sentry.OpenTelemetry.Exporter#5217
feat: Propagate W3C + Sentry headers in Sentry.OpenTelemetry.Exporter#5217jamescrosswell wants to merge 1 commit into
Conversation
… in Sentry.OpenTelemetry.Exporter Fixes #5210 Resolves the adoption footgun where the OOTB default was Sentry-only propagation. The default is now a CompositeTextMapPropagator that includes both TraceContextPropagator (W3C traceparent/tracestate) and SentryPropagator (sentry-trace + baggage), enabling out-of-the-box interoperability with services that use standard W3C trace context headers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5217 +/- ##
==========================================
+ Coverage 74.13% 74.17% +0.03%
==========================================
Files 506 506
Lines 18292 18296 +4
Branches 3576 3576
==========================================
+ Hits 13561 13571 +10
+ Misses 3859 3854 -5
+ Partials 872 871 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thank you sir 🫡 |
| } | ||
|
|
||
| defaultTextMapPropagator ??= new SentryPropagator(); | ||
| defaultTextMapPropagator ??= new CompositeTextMapPropagator(new TextMapPropagator[] |
There was a problem hiding this comment.
question: propagateTraceparent option
Should this option be guarded by the SentryOptions.PropagateTraceparent?
Is this a preceding oversight? Or should
Note: This option is currently only used in the base SentryMessageHandler (derived: SentryHttpMessageHandler and SentryGraphQLHttpMessageHandler)
There was a problem hiding this comment.
follow-up question: docs
In the docs we mention
The following SDKs support the
propagateTraceparentoption:
see https://docs.sentry.io/concepts/otlp/sentry-with-otel/
Should we list .NET there too?
There was a problem hiding this comment.
Should this option be guarded by the
SentryOptions.PropagateTraceparent? Is this a preceding oversight? Or shouldNote: This option is currently only used in the base
SentryMessageHandler(derived:SentryHttpMessageHandlerandSentryGraphQLHttpMessageHandler)
Hm, interesting question. According to the docs:
The integration MUST NOT set up an automatic propagator. Cross-service trace propagation is handled by the propagateTraceparent setting or by user-configured OTel propagators. How to set up an OTel propagator is a documentation concern.
So even the previous behaviour we had doesn't seem consistent with the docs. When using OTLP, presumably users can and should configure the propagators via OTEL (not Sentry):
- SDK users should call
Sdk.SetDefaultTextMapPropagatorto configure the OTEL propagation settings - We should stop calling that in our code:
- We should make the above clear in the docs for the OTLP integration
@ericsampson this is a bit different to what you were expecting... but it's also how all of our other OTLP integrations behave so I don't think we have much wiggle room on this one.
There was a problem hiding this comment.
|
We shouldn't actually be implementing this. See: |
Fixes #5210
Summary
TextMapPropagatorused byAddSentryOtlpExporter(andUseOtlp) was previouslySentryPropagator, meaning onlysentry-traceandbaggageheaders were propagated out of the box.CompositeTextMapPropagatorcontaining bothTraceContextPropagator(W3Ctraceparent/tracestate) andSentryPropagator(sentry-trace+baggage).defaultTextMapPropagatorare unaffected — the new default only applies when no propagator is supplied.🤖 Generated with Claude Code